Skip to content

Conversation

@jeremyd2019
Copy link
Contributor

In case of an error, the DL_info struct may have been left uninitialized, so it is not safe to use its members.

In one error case, initialize dli_sname to nullptr explicitly, so that the later check against nullptr is guaranteed to be safe.

@llvmbot
Copy link
Member

llvmbot commented May 3, 2025

@llvm/pr-subscribers-llvm-support

Author: None (jeremyd2019)

Changes

In case of an error, the DL_info struct may have been left uninitialized, so it is not safe to use its members.

In one error case, initialize dli_sname to nullptr explicitly, so that the later check against nullptr is guaranteed to be safe.


Full diff: https://github.com/llvm/llvm-project/pull/138369.diff

1 Files Affected:

  • (modified) llvm/lib/Support/Unix/Signals.inc (+20-13)
diff --git a/llvm/lib/Support/Unix/Signals.inc b/llvm/lib/Support/Unix/Signals.inc
index 691e1014f18e8..b2bc76a1121d4 100644
--- a/llvm/lib/Support/Unix/Signals.inc
+++ b/llvm/lib/Support/Unix/Signals.inc
@@ -826,14 +826,17 @@ void llvm::sys::PrintStackTrace(raw_ostream &OS, int Depth) {
   int width = 0;
   for (int i = 0; i < depth; ++i) {
     Dl_info dlinfo;
-    dladdr(StackTrace[i], &dlinfo);
-    const char *name = strrchr(dlinfo.dli_fname, '/');
-
     int nwidth;
-    if (!name)
-      nwidth = strlen(dlinfo.dli_fname);
-    else
-      nwidth = strlen(name) - 1;
+    if (dladdr(StackTrace[i], &dlinfo) == 0) {
+      nwidth = 7; // "(error)"
+    } else {
+      const char *name = strrchr(dlinfo.dli_fname, '/');
+
+      if (!name)
+        nwidth = strlen(dlinfo.dli_fname);
+      else
+        nwidth = strlen(name) - 1;
+    }
 
     if (nwidth > width)
       width = nwidth;
@@ -841,15 +844,19 @@ void llvm::sys::PrintStackTrace(raw_ostream &OS, int Depth) {
 
   for (int i = 0; i < depth; ++i) {
     Dl_info dlinfo;
-    dladdr(StackTrace[i], &dlinfo);
 
     OS << format("%-2d", i);
 
-    const char *name = strrchr(dlinfo.dli_fname, '/');
-    if (!name)
-      OS << format(" %-*s", width, static_cast<const char *>(dlinfo.dli_fname));
-    else
-      OS << format(" %-*s", width, name + 1);
+    if (dladdr(StackTrace[i], &dlinfo) == 0) {
+      OS << format(" %-*s", width, "(error)");
+      dlinfo.dli_sname = nullptr;
+    } else {
+      const char *name = strrchr(dlinfo.dli_fname, '/');
+      if (!name)
+        OS << format(" %-*s", width, static_cast<const char *>(dlinfo.dli_fname));
+      else
+        OS << format(" %-*s", width, name + 1);
+    }
 
     OS << format(" %#0*lx", (int)(sizeof(void *) * 2) + 2,
                  (unsigned long)StackTrace[i]);

@jeremyd2019
Copy link
Contributor Author

As promised to @mstorsjo in #138117 (comment).

On success, these functions return a nonzero value.

@github-actions
Copy link

github-actions bot commented May 3, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@jeremyd2019 jeremyd2019 force-pushed the check-dladdr-returns branch from dcd5e18 to 9fc9b53 Compare May 3, 2025 00:29
In case of an error, the DL_info struct may have been left
uninitialized, so it is not safe to use its members.

In one error case, initialize dli_sname to nullptr explicitly, so that
the later check against nullptr is guaranteed to be safe.

Signed-off-by: Jeremy Drake <[email protected]>
@jeremyd2019 jeremyd2019 force-pushed the check-dladdr-returns branch from 9fc9b53 to 3c25a22 Compare May 3, 2025 00:39
Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jeremyd2019
Copy link
Contributor Author

do I need to do something to get CI to run again? It doesn't seem to have failed due to anything in my control

@mstorsjo
Copy link
Member

mstorsjo commented May 7, 2025

do I need to do something to get CI to run again? It doesn't seem to have failed due to anything in my control

Yeah the failure seems to be unrelated. Some people have access to trigger retries on builkite, but those jobs should be switched over to github actions quite soon (where it should be easier to restart them), so that's mostly a moot point.

Anyway, I didn't mean to hold this one up, I just seem to have lost track of it and forgotten to merge it.

@mstorsjo mstorsjo merged commit 79bc8ad into llvm:main May 7, 2025
9 of 11 checks passed
@jeremyd2019 jeremyd2019 deleted the check-dladdr-returns branch May 7, 2025 19:40
@mstorsjo
Copy link
Member

mstorsjo commented May 7, 2025

Btw, unrelated to this particular patch; it seems like you don't have a realname set on your github account. When merging patches via the "squash and merge" method, the patches get rewritten to use the name from the account, not the (proper) name as it was in the individual git comits - so the patches end up with just Author: jeremyd2019 <[email protected]>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants